Skip to content

Conversation

@syntrydy
Copy link
Contributor

@syntrydy syntrydy commented Nov 18, 2025

#2449
Closes #2453

Summary by CodeRabbit

  • New Features

    • Editable API Configuration page with form, commit modal, and success/error toasts.
    • Audit logging for configuration updates (audit failures show warnings but don't block saves).
  • Refactor

    • Improved page load and submit flow with clearer error handling and retry-aware behavior.
    • Stronger type safety across configuration editors and form components.
  • Other

    • TypeScript support for importing YAML files.

✏️ Tip: You can customize this high-level summary in your review settings.

@syntrydy syntrydy requested a review from moabu November 18, 2025 21:40
@syntrydy syntrydy self-assigned this Nov 18, 2025
@syntrydy syntrydy requested a review from duttarnab as a code owner November 18, 2025 21:40
@syntrydy syntrydy added comp-admin-ui Component affected by issue or PR comp-docker-admin-ui Component affected by issue or PR labels Nov 18, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 18, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Replaces Redux-based Config API wiring with TypeScript hook-based clients and prop-driven components: removes Redux slice, saga, and API wrapper; adds typed hooks, type re-exports, YAML ambient declarations, refactors form and JSON property builder to TypeScript, and updates plugin metadata. (31 words)

Changes

Cohort / File(s) Summary
YAML Type Declarations
admin-ui/app/types/yaml.d.ts
Adds ambient module declarations for *.yaml and *.yml, introducing YamlModuleContent with optional components.schemas and an index signature.
Form Component Refactor
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Converts ApiConfigForm to a prop-driven React.FC<ApiConfigFormProps>: accepts configuration and onSubmit(patches, message), manages local modal/patch state, permission checks, and submit flow via callback.
Page Component Migration
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.js, admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
Removes legacy JS Redux page and adds a new TS page using useGetConfigApiProperties, usePatchConfigApiProperties, and useConfigApiActions; implements loading, error handling, patching, and audit logging.
JSON Property Builder Typing
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/JsonPropertyBuilderConfigApi.tsx
Replaces PropTypes with TypeScript interfaces; adds explicit type guards, uses initialPath, unique IDs, stronger indexing via Record<string, unknown>, and JsonPatch annotations.
Hooks & Audit Logging
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts, admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/index.ts
Adds useConfigApiActions hook exposing logConfigApiUpdate and ModifiedFields interface; re-exports hook from hooks index.
Type Definitions & Barrel
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts, .../componentTypes.ts, .../types/index.ts
Adds re-exports for ApiAppConfiguration and JsonPatch; defines SchemaProperty and JsonPropertyBuilderConfigApiProps; creates a barrel export.
Redux Layer Removal
admin-ui/plugins/auth-server/redux/api/ConfigurationApi.js, admin-ui/plugins/auth-server/redux/features/configApiSlice.js, admin-ui/plugins/auth-server/redux/sagas/configApiSaga.js
Deletes ConfigurationApi class, Redux slice (actions/reducer/registration), and sagas/watchers — removing prior fetch/patch/audit saga logic.
Plugin Metadata Update
admin-ui/plugins/auth-server/plugin-metadata.js
Removes configApiReducer and configApiSaga entries from plugin metadata reducers and sagas lists.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ConfigApiPage as ConfigApiPage (New)
    participant ApiConfigForm
    participant Hooks as Custom Hooks
    participant API as API Layer

    User->>ConfigApiPage: Mount
    ConfigApiPage->>Hooks: useGetConfigApiProperties()
    Hooks->>API: GET /config/api
    API-->>Hooks: configuration
    Hooks-->>ConfigApiPage: data + isLoading
    ConfigApiPage->>ApiConfigForm: render with configuration

    User->>ApiConfigForm: submit(patches,message)
    ApiConfigForm->>ConfigApiPage: onSubmit(patches,message)
    ConfigApiPage->>Hooks: usePatchConfigApiProperties(patches)
    Hooks->>API: PATCH /config/api
    API-->>Hooks: success / error

    alt success
        Hooks-->>ConfigApiPage: success
        ConfigApiPage->>Hooks: useConfigApiActions().logConfigApiUpdate(...)
        Hooks->>API: POST /audit
        ConfigApiPage-->>User: show success toast
    else error
        Hooks-->>ConfigApiPage: error
        ConfigApiPage-->>User: show error card/toast
    end
Loading
sequenceDiagram
    participant OldFlow as Redux Flow (Removed)
    participant NewFlow as Hooks Flow (New)

    rect rgb(240,220,220)
    Note over OldFlow: Dispatch -> Saga -> API -> Reducer -> Component select
    OldFlow->>OldFlow: dispatch(getConfigApiConfiguration)
    OldFlow->>OldFlow: saga handles fetch + audit
    OldFlow->>OldFlow: dispatch(response) -> update slice
    end

    rect rgb(220,240,220)
    Note over NewFlow: Hook-based direct data + local state
    NewFlow->>NewFlow: useGetConfigApiProperties() -> returns data/isLoading
    NewFlow->>NewFlow: onSubmit -> usePatchConfigApiProperties -> audit via useConfigApiActions
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas needing extra attention:
    • Hook implementations and integration with the generated TypeScript client and their error/refresh semantics.
    • Ensuring all removed Redux artifacts have no remaining imports/usages across the codebase.
    • Type guards and casts in JsonPropertyBuilderConfigApi for nested property access and runtime behavior.
    • Audit logging payload shape and non-fatal error handling in ConfigApiPage.

Possibly related PRs

Suggested reviewers

  • duttarnab

Poem

🐰 I hopped through types and trimmed old sagas' vines,

YAML whispers neat, hooks wag tiny lines,
Patches stitched with care, audits softly sung,
Forms now typed and tidy — a rabbit’s little sprung,
Cheers to cleaner code where new hooks run.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: replacing manually created types with a TypeScript generated client for Auth Server Config API pages.
Linked Issues check ✅ Passed The PR successfully addresses all objectives from issue #2453: replaces manually created types with TypeScript generated client, removes redux artifacts (api, features, saga), and eliminates manual types.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the linked issue objectives. New files (types, hooks, components) are introduced to support TypeScript client integration, while old redux and manual type files are removed as specified.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05a5cf4 and fcef427.

📒 Files selected for processing (1)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto mo-auto added the kind-bug Issue or PR is a bug in existing functionality label Nov 18, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/JsonPropertyBuilderConfigApi.tsx (1)

11-37: Schema-based type guards risk unsound narrowing; consider loosening the TS predicates.

The helper functions _isBoolean, _isString, and isStringArray act as TS type guards but also treat the JSON schema as authoritative:

function _isBoolean(item: unknown, schema?: JsonPropertyBuilderConfigApiProps['schema']): item is boolean {
  return typeof item === 'boolean' || schema?.type === 'boolean'
}

Because schema?.type === 'boolean' doesn’t inspect the actual runtime value, TypeScript will happily treat propValue as a boolean even if it’s still a string like 'true'. The same applies to strings and string arrays, which can lead to incorrect assumptions about propValue’s shape inside the guarded branches.

To keep the runtime behavior while avoiding misleading narrowing, you could:

  • Keep the current logic but change the return type of these helpers from item is ... to boolean, or
  • Split runtime type checks from schema checks, e.g. a true type guard for the value and a separate isBooleanFieldBySchema function used purely for rendering decisions.

Also, _isNumber currently treats bigint as a number in its predicate; if propValue can never be a bigint from this API, you might simplify it to only accept typeof item === 'number' to keep the guard strictly accurate.

admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)

81-98: Typed access into configuration/schema works; you can tighten key typing

The cast configuration[propKey as keyof ApiAppConfiguration] and the narrowed schema[propKey] are reasonable, but you can make this safer and avoid the cast by typing the keys:

-      {Object.keys(configuration).map((propKey) => {
+      {(Object.keys(configuration) as (keyof ApiAppConfiguration)[]).map((propKey) => {
         if (generateLabel(propKey)) {
           return (
             <JsonPropertyBuilderConfigApi
               key={propKey}
               propKey={propKey}
-              propValue={configuration[propKey as keyof ApiAppConfiguration]}
+              propValue={configuration[propKey]}
               lSize={6}
               handler={patchHandler}
               schema={
-                schema[propKey] as { type?: string; items?: { type?: string; enum?: string[] } }
+                schema[propKey as string] as {
+                  type?: string
+                  items?: { type?: string; enum?: string[] }
+                }
               }

This keeps TS aligned with the actual ApiAppConfiguration keys and minimizes unsafe casting.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7dd724e and 38e803b.

📒 Files selected for processing (14)
  • admin-ui/app/types/yaml.d.ts (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (3 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.js (0 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/JsonPropertyBuilderConfigApi.tsx (9 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/index.ts (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/componentTypes.ts (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/index.ts (1 hunks)
  • admin-ui/plugins/auth-server/plugin-metadata.js (0 hunks)
  • admin-ui/plugins/auth-server/redux/api/ConfigurationApi.js (0 hunks)
  • admin-ui/plugins/auth-server/redux/features/configApiSlice.js (0 hunks)
  • admin-ui/plugins/auth-server/redux/sagas/configApiSaga.js (0 hunks)
💤 Files with no reviewable changes (5)
  • admin-ui/plugins/auth-server/plugin-metadata.js
  • admin-ui/plugins/auth-server/redux/api/ConfigurationApi.js
  • admin-ui/plugins/auth-server/redux/sagas/configApiSaga.js
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.js
  • admin-ui/plugins/auth-server/redux/features/configApiSlice.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
🔇 Additional comments (8)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)

9-38: Audit hook wiring looks correct; verify payload sensitivity and simplify ModifiedFields.

The hook is cleanly wired: it derives auth context from Redux and exposes a focused logConfigApiUpdate helper which callers can wrap in their own error handling.

Two points to consider:

  • If ApiAppConfiguration includes any sensitive fields (e.g. client secrets, tokens), logging the full object as payload might be more than you need. It may be safer to either:
    • log only the minimal subset of fields required for auditability, or
    • ensure logAuditUserAction centrally redacts sensitive keys before persistence.
  • ModifiedFields can be expressed more idiomatically as a type alias:
-export interface ModifiedFields {
-  [key: string]: unknown
-}
+export type ModifiedFields = Record<string, unknown>
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/index.ts (1)

1-1: Hooks barrel is concise and correct.

Re-exporting useConfigApiActions here keeps the Config API hooks discoverable without adding complexity.

admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/index.ts (1)

1-2: Type barrel is well-structured.

Aggregating configApiTypes and componentTypes behind this index keeps downstream imports clean and localized to a single entry point.

admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)

1-3: Type re-exports correctly encapsulate JansConfigApi.

This file cleanly centralizes the external ApiAppConfiguration and JsonPatch types, which makes it easier to adjust if the generated client ever changes its signatures.

admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1)

20-52: Config API page wiring looks solid; please verify mutation/query integration with the generated client.

The page composes the generated hooks and the new audit hook cleanly:

  • Initial load via useGetConfigApiProperties.
  • Updates via usePatchConfigApiProperties().mutateAsync({ data: patches }).
  • Non-fatal audit logging via logConfigApiUpdate(updatedConfig, message, { requestBody: patches }).
  • Clear separation between initial-load errors (error) and save-time errors (errorMessage + toasts).

Two things worth double‑checking against the generated JansConfigApi hooks:

  1. That usePatchConfigApiProperties either:
    • invalidates or updates the underlying useGetConfigApiProperties query so configuration stays in sync after a save, or
    • otherwise ensures the next render reflects the updated configuration.
  2. That the JsonPatch type imported from './types' matches the expected request body shape for usePatchConfigApiProperties (i.e., data: JsonPatch[] is exactly what the generated client expects).

If both are true, this page should behave as intended without additional glue code.

admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (3)

6-11: Imports for API_CONFIG_WRITE and typed config/patch models look consistent

API_CONFIG_WRITE is correctly used both in authorize and permission checks, and the ApiAppConfiguration / JsonPatch type imports align with the new prop-driven API. No issues here.


37-47: Updated useEffect dependency on authorize is appropriate

Using [authorize] in the dependency array avoids the stale-closure pattern while still effectively behaving like a “run on mount” effect for a stable hook API. The authorization error logging is fine for now.


57-63: Consider clearing patches/operations after a successful submit

Delegating to onSubmit(patches, userMessage) is correct, but patches and operations are never cleared. If this form stays mounted after a successful update, any subsequent changes will send both the new and all previously applied patches again.

To avoid reapplying stale operations, you can clear state after a successful submit:

  const submitForm = useCallback(
    async (userMessage: string) => {
      toggle()
-      await onSubmit(patches, userMessage)
+      await onSubmit(patches, userMessage)
+      setPatches([])
+      setOperations([])
    },
-    [toggle, onSubmit, patches],
+    [toggle, onSubmit, patches],
  )

If the parent unmounts/remounts ApiConfigForm after submit, this is less critical, but it’s safer to handle cleanup locally.

Copy link
Contributor

@duttarnab duttarnab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. In main branch ,the resource name was 'Config API configuration'
  2. In main branch, payload shows the patch request of the modified fields.
    The above two things are not matching in the PR's branch
image

@syntrydy
Copy link
Contributor Author

@duttarnab

On point one: Current: "Config API Configuration" (uppercase 'C') Target: "Config API configuration" (lowercase 'c')
Is this what you are refering to?
On point two: PATCH Request Sending All Fields Instead of Only Modified Fields, is that the problem you are pointing?

@syntrydy syntrydy requested a review from duttarnab November 19, 2025 18:07
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19372ae and 18380be.

📒 Files selected for processing (1)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAuditUserAction (25-66)
🔇 Additional comments (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (3)

9-11: LGTM!

The ModifiedFields interface provides appropriate flexibility for tracking varied field changes in audit logs.


13-18: LGTM!

The Redux selector logic correctly extracts authentication context needed for audit logging.


36-39: LGTM!

The return structure follows standard React hook patterns.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18380be and 8858f1e.

📒 Files selected for processing (2)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1 hunks)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts
🧬 Code graph analysis (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
  • useConfigApiActions (11-36)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
  • JsonPatch (3-3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAuditUserAction (25-66)
🔇 Additional comments (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1)

49-71: Loading/error and render flow looks solid

Combining query and mutation flags into a single loading prop for GluuLoader, short‑circuiting on initial error with an inline card, and conditionally rendering ApiConfigForm plus an errorMessage alert gives a clear UX and keeps concerns separated. No issues here.

admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)

7-35: Audit logging hook implementation is correct and prior issues are resolved

useConfigApiActions now cleanly derives token, client_id, and userinfo from authReducer and wraps logAuditUserAction with the right metadata (UPDATE, resource "Config API configuration", performedOn: 'config-api', and modifiedFields without overriding payload). The previous problems with an unused config parameter and conflicting payload construction are gone, and the useCallback dependency list matches the captured values.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8858f1e and 5f127d6.

📒 Files selected for processing (1)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
  • useConfigApiActions (11-36)
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
  • errorMessage (100-100)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
  • JsonPatch (3-3)
🔇 Additional comments (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1)

13-23: Config page flow and audit handling look correct and address prior gating concern

The overall wiring (fetch via useGetConfigApiProperties, PATCH via usePatchConfigApiProperties, then non-blocking logConfigApiUpdate with a warning toast on audit failure) is solid and fixes the earlier issue where audit logging depended on the response body. Keeping console.error in both catch blocks is also good for debugging when multiple operations happen in the same try.

Based on learnings

Also applies to: 24-44, 58-69

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f127d6 and 05a5cf4.

📒 Files selected for processing (1)
  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.

Applied to files:

  • admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
  • useConfigApiActions (11-36)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
  • JsonPatch (3-3)
🔇 Additional comments (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (3)

1-22: LGTM! Clean setup with proper TypeScript typing.

The imports and component initialization follow React best practices. The hooks are properly initialized, types are imported, and the i18n setup is correct.


46-56: LGTM! Proper loading state combination and improved error handling.

The loading state correctly combines both the mutation pending state and the initial query loading state. The error handling now properly extracts the error message using instanceof Error check, addressing the previous review concern about String(error) potentially rendering [object Object].


58-69: LGTM! Proper render flow with both transient and persistent error feedback.

The component correctly:

  • Wraps content in GluuLoader for loading states
  • Conditionally renders the form only when configuration data is available
  • Provides both immediate feedback via toast notifications and persistent error display via the alert div

The dual error display (toast + persistent alert) is a good UX pattern that provides immediate feedback while maintaining visibility of errors after the toast disappears.

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@moabu moabu merged commit f0ac17b into main Nov 20, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-admin-ui Component affected by issue or PR comp-docker-admin-ui Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(admin-ui): Use typescript generated client for Auth Server Config API pages

4 participants